Skip to content

Fix #4446: Inline implementation of PF methods into its anonymous class #4604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 13, 2018

Conversation

allanrenucci
Copy link
Contributor

No description provided.

@allanrenucci
Copy link
Contributor Author

allanrenucci commented May 31, 2018

Blocked on #1895.

@allanrenucci allanrenucci force-pushed the fix-4446 branch 3 times, most recently from e13cd9f to 6bafeb0 Compare June 1, 2018 11:51
smarter added a commit to dotty-staging/dotty that referenced this pull request Jun 1, 2018
When we create a symbol in a DenotationTransformer, we cannot safely
create it at phase `thisPhase.next` since this can lead to cycles. This
is why we created extension methods at phase `thisPhase`, but they're
not really valid at that point since they're only entered in their owner
once the transformer has been run. This lead to stale symbol errors
which were worked around in 51a458e but
the fix is incomplete and does not work with scala#4604 because that PR moves
the ExtensionMethods phase around.

This commit removes all the workarounds and uses a simpler solution:
create the symbols at phase `thisPhase` but set their validity
period to match the validity of the owner transformed denotation.
@allanrenucci allanrenucci force-pushed the fix-4446 branch 3 times, most recently from 5ec2ab0 to 0025a60 Compare June 6, 2018 14:26
@allanrenucci allanrenucci requested a review from smarter June 6, 2018 14:35
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to add a documentation comment to toPartialFunction that shows what transformation is being done with an example, a bit like the comment on top of https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/transform/VCInlineMethods.scala

@allanrenucci allanrenucci merged commit b35309c into scala:master Jun 13, 2018
@allanrenucci allanrenucci deleted the fix-4446 branch June 13, 2018 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants